[automation] Add support for synchronized execution of compiled scripts #4402
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e implements Lock interface Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
J-N-K
reviewed
Oct 5, 2024
.../org/openhab/core/automation/module/script/internal/handler/AbstractScriptModuleHandler.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
|
@J-N-K I adressed your review 👍 |
florian-h05
changed the title
|
FYI this is for openhab/openhab-addons#17510. |
|
Jenkins finally finished the PR build. |
J-N-K
requested changes
Oct 6, 2024
Comment on lines
214
to
223
| return compiledScript.get().eval(engine.getContext()); | ||
| Object result; |
There was a problem hiding this comment.
Why do you create a new object here? You could also return it directly and move the return null from the ScriptException catch-block to the end of method (or return nul in the finally block.
There was a problem hiding this comment.
It "feels" better this way for me when reading the code, but I have changed it.
Comment on lines
215
to
216
| if (engine instanceof Lock lock) { | ||
| if (!lock.tryLock(1, TimeUnit.MINUTES)) { |
There was a problem hiding this comment.
Suggested change
| if (engine instanceof Lock lock) { | |
| if (!lock.tryLock(1, TimeUnit.MINUTES)) { | |
| if (engine instanceof Lock lock && !lock.tryLock(1, TimeUnit.MINUTES)) { |
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
florian-h05
added a commit
to florian-h05/openhab-core
that referenced
this pull request
Oct 26, 2024
…implements Lock This moves the locking mechanism added in openhab#4402 to the inheritors of AbstractScriptModuleHandler to synchronize execution context access as well. This fixes the problem thathttps://github.com/openhab/openhab-addons/pull/17510 worked around by using Thread.sleep. Signed-off-by: Florian Hotze <dev@florianhotze.com>
kaikreuzer
pushed a commit
that referenced
this pull request
Oct 26, 2024
…implements Lock (#4426) This moves the locking mechanism added in #4402 to the inheritors of AbstractScriptModuleHandler to synchronize execution context access as well. This fixes the problem thathttps://github.com/openhab/openhab-addons/pull/17510 worked around by using Thread.sleep. Signed-off-by: Florian Hotze <dev@florianhotze.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If the ScriptEngine implements the Lock interface, the execution of the CompiledScript is synced by acquisition and release of the log.
This is required for add-ons like JS Scripting.